Skip to content

feat(wallet): Safe multi-owner accounts - #160

Open
flotob wants to merge 30 commits into
feature/openlvfrom
feature/safe-accounts
Open

feat(wallet): Safe multi-owner accounts#160
flotob wants to merge 30 commits into
feature/openlvfrom
feature/safe-accounts

Conversation

@flotob

@flotob flotob commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Safe (multisig) smart accounts as first-class wallet accounts: create, activate, send, and use as a dApp wallet — with every owner type the browser already speaks (vault, Ledger, phone via openlv) co-signing through the existing signer seam. Stacked on #159 (openlv), which is stacked on #149 (Ledger).

What's in here

Account model + creation. A Safe is an account record whose owners are existing wallet records; presets "backup (1 of 2)" and "resilient (2 of 3)" only — no 2/2 (lose either device and funds are stuck). The record freezes the original init params (owners/threshold/saltNonce) so the CREATE2 address is reproducible on any chain; v1 deploys on Gnosis. Counterfactual receive-only state with first-class blocking states: activation quote, "fund the executor", no local executor.

Execution layer. @safe-global/protocol-kit as a plain dependency against the user's own RPC pool — never Safe's hosted Transaction Service. Deployments only through the canonical safe-deployments factory. An executor EOA (first mnemonic owner) pays gas; the review screens say who pays.

Asynchronous signing board. Collecting owner signatures is a user-paced task, not a pipeline: one row per owner with its own action (vault signs free and silently, "Sign with Ledger" with warm detection, "Show QR code" for the phone), row failures never fail the transaction, rejection is a decision, everything is persisted per-signature and survives restarts. Execution is a separate idempotent, Safe-nonce-guarded step that auto-runs at threshold. A wallet-wide "Unfinished transactions" row (with count badge) resumes any parked transaction. A locked vault anywhere in the flow walks through the standard unlock screen instead of erroring.

dApp integration (EIP-1271). Deployed safes are connectable to dApps; personal_sign/eth_signTypedData_v4 are answered with owner signatures over the SafeMessage envelope (instant when free vault signatures meet the threshold, otherwise the board), and verifying dApps call isValidSignature on the Safe — with honest UI caveats that some apps can't verify contract signatures. eth_sendTransaction routes through the board and resolves with the execution hash. Gnosis-only guards; safes are never auto-approved.

Also fixed en route: relay failover for phone signing (probes mosquitto → EMQX → HiveMQ; the chosen relay rides in the QR), payment-history rows for Safe sends say from = safe address with the executor in metadata, and the remote-session unit tests no longer depend on live public MQTT brokers.

Verification

  • ~2350 jest tests incl. an anvil-fork suite proving: identical addresses on forked Gnosis AND Base from the same init params, retroactive deployment claiming pre-sent funds, 2/3 execution through the real broadcast path, and isValidSignature accepting collected EIP-1271 signatures on the real contract (tampered digest refused).
  • Three Playwright E2Es against an anvil Gnosis fork (skip cleanly without anvil/network, e.g. in CI): full lifecycle through the real UI (safe-accounts), a phone owner co-signing 2/3 through the real bridge page over a local relay (safe-phone), and a real dApp page connecting a Safe, getting a verified 1271 signature, and sending through the board (safe-dapp).
  • Field-tested: 1/2 and 2/3 sends confirmed on real Gnosis with Ledger AND phone co-signatures; dApp signing smoke-tested.

🤖 Generated with Claude Code

meinharrd and others added 16 commits July 7, 2026 20:29
Bump PINNED_RELEASE_TAG v0.5.33 -> v0.5.36 together with
PINNED_SHA256SUMS_DIGEST (sha256 of the release's SHA256SUMS asset,
recorded at pin time).

Notable upstream changes since 0.5.33:
- 0.5.36: HTTP API + control socket bind before chain init (peer count
  visible from the first poll, chainReady flag in /health); socket-path
  failures non-fatal (solardev-xyz/ant#38, solardev-xyz/ant#39)
- 0.5.35: push-path throughput (~250 MiB upload stall fix), feed
  head-finding fix, concurrent chain init
- 0.5.34: upload-side Reed-Solomon encoding, Swarm content encryption
  end to end, local pinning, ACT access control, stewardship

Verified: ant:download installs and checksum-verifies all six targets,
check-binaries passes, bee-to-ant migration integration test passes
against the released binary, and a startup probe of the bundled binary
confirms /health answers 200 within 0.5s of spawn with live /peers
counts.
Freedom had no find-in-page. Add a Chrome-style overlay bar anchored to
the top-right of the webview area, driving the active tab's <webview>
via findInPage()/stopFindInPage() with match counts read back from the
found-in-page event.

- Cmd/Ctrl+F opens and focuses the bar (Edit-menu accelerator when the
  page has focus, renderer keydown fallback when the chrome does);
  prefills from the page's text selection when present
- Find-as-you-type (debounced), Enter / Shift+Enter cycle matches, Esc
  closes and clears highlights; zero matches shows "0/0" with a subtle
  error tint on the input
- Find state follows the foreground page: switching tabs closes the bar
  and clears the outgoing tab's highlights, navigating the active tab
  resets results while keeping the query for re-search on Enter
- New find:open IPC channel (main -> renderer) behind an explicit Edit
  menu item so the shortcut is discoverable; macOS keeps the editMenu
  role with a spelled-out submenu to append the item
- Dark/light theme support via the existing CSS variables

Tests: jest unit coverage for the find-bar state machine (mocked
webview) plus a Playwright harness spec covering match cycling, zero
matches, and tab-switch close.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Reverts 73ee4a0, which was pushed to main by accident (local
push.default=upstream resolved a bare-name push through the feature
branch's origin/main tracking ref). The feature re-lands through a
reviewed PR from feature/find-in-page.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A Safe account has no key of its own: its owners are existing wallet
records, and a SafeTx is EIP-712 typed data, so every signer backend
(vault, Ledger, phone) co-signs through the ordinary getSigner seam.
src/main/wallet/safe/safe-executor.js orchestrates the rest over
@safe-global/protocol-kit (local SDK only — never the hosted Safe
Transaction Service): counterfactual address prediction, deployment
through the canonical safe-deployments factory (asserted, never a
user-supplied one), SafeTx build/hash returning plain-JSON shapes,
sequential recover-verified owner-signature collection, and
execTransaction submitted by an executor EOA that pays the gas.

The record's original init params (owners, threshold, saltNonce) are
the reproducibility anchor: buildSafeTransaction re-derives the CREATE2
address from them and refuses on mismatch, and the anvil-fork
integration test (skips without anvil/network) proves the same params
deploy to the same address on forked Gnosis AND Base, with retroactive
deployment claiming funds sent to the address before it existed.

getSigner(safeIndex) now throws — a Safe is an account, not a signer.
provider-manager grows getEip1193Provider(chainId) (protocol-kit needs
a raw request interface FallbackProvider cannot give) sharing the RPC
pool and cache invalidation; transaction-service grows toFeeFields so
callers stop re-deriving the eip1559/legacy branch.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A Safe is now a first-class wallet record (type 'safe') whose owners
are existing accounts, created from the account dropdown with the two
shipped presets — backup (1 of 2) and resilient (2 of 3); 2-of-2 is
deliberately not offered since losing either device would brick the
funds. The record freezes the init params (owners, threshold,
saltNonce) that make the CREATE2 address reproducible; identity-manager
enforces the presets, refuses Safes owning Safes, and blocks deleting
an account while a Safe references it as owner.

Creation is free: safe-service predicts the counterfactual address and
stores the record, so the account can receive immediately. "Needs
funds" is a first-class blocking state, not an error — the status card
under Send/Receive quotes the one-time Gnosis activation (deployment
tx built once, gas + executor balance checked in parallel) and either
offers Activate, or blocks with "fund <executor> with ≥ X xDAI", or
explains that no owner can pay. Activation reuses the quoted
deployment tx, waits for confirmation, and only then marks the record
deployed (chain state also self-heals the record).

Until the multi-owner signing flow ships, Safe accounts are
receive-only: Send is disabled with an explanation and dApp connect
skips them (EIP-1271 comes later). The shared account picker grew
label/multi-select support so the owner list, Ledger, and phone
screens render from one component.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ence

A deployed Safe can now send: the send flow branches into a Safe path
where main builds the SafeTx, collects owner signatures one device at a
time (vault instantly, Ledger tap, phone QR — all through the existing
signer seam), and submits execTransaction through the executor EOA. The
pending view shows a per-owner signature checklist driven by progress
events streamed from main; the review screen names who pays the fee
instead of quoting gas that can only be known after signing.

Every collected signature is persisted the moment it exists (interim
JSON, one pending SafeTx per Safe — a single slot sidesteps the nonce
replacement swamp). A rejection, an unreachable phone, a failed
broadcast, or an app restart never loses signatures: the status card
shows "transaction awaiting signatures (1 of 2)" with continue/discard,
and resume skips owners who already signed.

Safe transactions land in payment history: safe-send rows say from =
safe address with the executor and safeTxHash in metadata (tx-recorder
gained a fromAddress override), and activation deploys record as
safe-deploy. The payments page knows both kinds.

Also fixes a stale-capability bug: the renderer's record snapshot now
syncs deployment truth from main when the status card refreshes, so the
Send button enables right after activation instead of after a restart.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
startSafeSend passed the safe record straight into the executor layer,
so protocol-kit received the owners as wallet INDEXES and viem died on
getAddress(0) ("Address \"0\" is invalid") the moment a safe send was
confirmed. The activation path resolved them correctly; the send
orchestrator now does the same, and initPredictedKit fails loudly on
non-address owners so the index/address mixup can never reach the
address parser again.

The unit test had asserted the wrong shape against a mocked builder —
fixed, and the anvil-fork suite gained an un-mocked regression test
that walks the real record → build → collect(2/3) → execTransaction
path and checks the funds arrive.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
New Playwright spec drives the actual app end to end: create a 1-of-2
Safe in the wizard, hit the needs-funds blocking state, fund the
executor on an anvil fork of Gnosis (the registry's user config points
every Gnosis RPC at it, builtin endpoints removed), activate through
the canonical factory, watch Send enable, and send 0.5 xDAI out of the
Safe — recipient balance verified on-chain. Skips cleanly without
anvil/network, like the jest fork suite. The fork runs with
--block-time so confirmation behaves like the real chain.

Two findings it caught immediately, both fixed:
- activating with a locked vault failed silently to the console and
  reset the button — the status card now surfaces the error inline;
- the Safe send path skipped gas estimation and with it the only check
  that an asset was selected — validateAmount now owns that check.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Field testing a 2/3 Safe showed the flaw: signature collection ran as a
pipeline that marched through owners in record order and declared
"Transaction Failed" the moment the Ledger wasn't plugged in — for a
task that is fundamentally asynchronous and belongs to the user, not
the app.

Collecting signatures is now a board the user drives. Confirming a send
creates and persists the SafeTx, silently adds the free signatures
(mnemonic owners, vault unlocked — kept, that's what made 1-of-2 feel
instant), and opens a dedicated "Collect signatures" subscreen: what is
being sent and to whom, "1 of 2 signatures — any 2 of the 3 owners can
sign", when it started, and one row per owner with its own action
(Sign with Ledger / Show QR code) that the user taps when the device is
actually in hand. A failed attempt is a row state with main's error
message; a rejection is a decision — the row quietly returns to
waiting. The board is leaveable: the account card shows what's waiting
("Sending 0.5 xDAI to 0x12…cd — 1 of 2 signatures, started 2 days
ago") and re-opens it. While the board is open, an unplugged Ledger row
flips to "Ledger detected — sign now" the moment it's connected. The
phone QR overlay now says it's approving your own multi-owner
transaction instead of dApp copy.

Main's API turned granular and defensive: start (build + free
signatures only — devices are never cold-called), sign-one-owner
(per-safe mutual exclusion; a signature landing after its transaction
was discarded is dropped by a safeTxHash identity check), and execute
as its own idempotent step — auto-run by the board at threshold,
nonce-guarded so a broadcast that secretly landed (or an
app.safe.global execution) flips the transaction to a truthful terminal
"superseded" state instead of retry-looping, with an executor-can't-pay
banner and signatures that survive every failure. The progress event
stream is gone; the board renders from returned state.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Pending SafeTxs move out of the account status card into a wallet-wide
"Unfinished transactions" row above Recent payments, with a count badge
across ALL Safe accounts. It opens an overview subscreen (one entry per
waiting transaction: summary, which Safe, signature progress, age) and
each entry opens its signing board — with a single pending transaction
the row jumps straight to the board. The status card is back to
activation states only.

Also fixes the raw-wei summary ("Sending 2000000000000000 to …") that
pending transactions created before the presentation fields existed
rendered with: the summary now formats atomic amounts with the stored
decimals and falls back to xDAI for native sends.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Field report: tapping "Show QR code" on the signing board failed within
a split second with "Phone signing failed. Try again." A new E2E — a
phone owner co-signing a 2/3 SafeTx through the real bridge page over a
local relay — passes, which isolated the failure to the environment:
the default signaling relay (test.mosquitto.org's public TEST broker)
currently accepts TLS and instantly hangs up the MQTT websocket,
reproducible with a raw probe.

The session broker now resolves its relay per session: an explicit
override (env) is used as-is, otherwise the public candidates
(mosquitto, EMQX, HiveMQ) are probed with a single websocket handshake
and the first reachable one wins (cached for a minute). The chosen
relay rides inside the QR, so the phone always joins the same one —
and relays only ever carry ciphertext. Non-Error job failures also stop
collapsing into the generic registry message, so the next environmental
failure names itself.

The new safe-phone E2E stays: it covers the signing board's QR row end
to end (vault free signature + phone co-signature + execution, funds
verified on the fork).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Field report: after collecting both signatures the board said "Vault is
locked / The signatures are kept — you can try again" — the vault had
auto-locked while the phone signature was being gathered, and executing
needs the executor's vault key. A locked vault is a step in the flow,
not an error: the safe IPC handlers now tag vault-locked failures with
a stable code, and the signing board (execute AND per-owner sign) plus
the activation card respond by opening the standard vault-unlock
screen and retrying the step after a successful unlock. Cancelling the
unlock leaves an actionable notice instead of a dead end.

The safe-accounts E2E now exercises this path for real: it clicks
Activate with a locked vault, unlocks through the UI, and waits on the
Send button (the deployed-truth barrier — the status card sits inside
the identity view, which the unlock screen hides, so a card-hidden
assertion passes prematurely). The phone E2E tolerates the card
quoting before or after the executor funding lands.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A dApp's personal_sign / eth_signTypedData_v4 against a Safe account is
answered with owner signatures over the SafeMessage EIP-712 envelope;
completeSafeMessage returns the sorted concatenated bytes the dApp
verifies via isValidSignature on the Safe.

- safe/safe-messages.js: in-memory sessions (a dApp request is a live
  promise that dies with its page); same-hash restart resumes with the
  collected signatures, a different hash replaces the dead session.
  Digests are computed with ethers over normalized input — protocol-kit's
  hashSafeMessage would UTF-8-hash hex personal_sign payloads, diverging
  from what EOA signers and verifying dApps compute.
- safe/signature-collection.js: the owner-collection machinery (in-flight
  lock, free-signature sweep, per-owner ceremony with identity re-check)
  extracted from safe-transactions over a store adapter; sends and
  message sessions share one lock per Safe.
- safe/errors.js: the SAFE_* code registry.
- IPC wallet:safe-message-start/-sign/-state/-cancel/-complete (lazy,
  VAULT_LOCKED-tagged) + window.wallet.safeMessage*.
- Fork test: collected signatures pass isValidSignature(bytes32,bytes)
  on the real deployed contract for personal_sign AND typed data; a
  tampered digest is refused.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- dapp-connect lists DEPLOYED safes (receive-only stay hidden), defaults
  to the active account, and says upfront that a multi-owner account
  signs as a smart contract some apps can't verify (same caveat on the
  sign approval screen).
- Message signing routes through the signing board's new 'message' mode:
  free vault signatures that meet the threshold answer instantly (no
  board), otherwise the board collects per-owner and completion hands
  the combined signature back and closes — closing/cancelling rejects
  the dApp with 4001. Opening the board over a live message session
  settles that session first.
- dApp eth_sendTransaction starts a pending SafeTx and hands over to the
  board; the fee row names who pays (getSafeStatus executor — also used
  by the send review now); the dApp promise resolves with the execution
  hash via wallet:safe-executed and rejects on wallet:safe-discarded.
  Parking the board keeps the dApp waiting — its transaction IS pending.
- Gnosis-only guards on both paths (an unverifiable signature or a
  never-executable SafeTx beats a confusing failure later); safes are
  never auto-approved for transactions.
- Board openers accept the caller's fresh state, dropping the redundant
  re-fetch + free-sweep IPC on every dApp interaction.
- SafeMessage phone signatures get their own QR copy (context
  'safe-message').
- NEW E2E safe-dapp.spec.js: a real bzz:// dApp page connects the Safe
  through the webview provider bridge, personal_sign verifies via
  isValidSignature on the anvil Gnosis fork, and a send comes back as an
  execution hash with the balance moved.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Without an injected signaling override the broker probes the public
MQTT brokers with a real WebSocket — the unit suite silently depended
on broker reachability. The harness now injects a fixed relay.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
simon-jentzsch and others added 3 commits July 10, 2026 16:07
This uses the 64bit SP1 v6 zk-proof and once every one has updated, we can stop generating the old v5 proofs.
Update colibri-stateless to version 2.0.0
@flotob
flotob requested a review from meinharrd July 12, 2026 08:59

@flotob flotob left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adversarial review of head d8db024: 2 blocking state-isolation findings. CI is green and the stacked branch is clean, but it also inherits #159 and its unresolved production bridge-origin blocker.

  • [P1] Identify Safe message sessions by request and caller, not only Safe index/hash (src/main/wallet/safe/safe-messages.js:134). Two live dApp tabs can request signatures for the same Safe. An identical digest resumes the other site session and can return its signature; a different digest silently replaces a still-live request and leaves its promise/UI attached to the wrong ceremony. Bind sessions to an unguessable request id plus requesting webContents/origin, reject or queue concurrent live requests, and require that token on state/sign/complete/cancel IPC.
  • [P2] Do not leave pending Safe state behind when deleting the Safe (src/main/identity-manager.js:1253). safe-pending.json is keyed by wallet index. Deleting a Safe leaves its entry; when the deleted Safe held the highest index, the next account can reuse that index and inherit or be blocked by the old SafeTx. Require explicit discard before removal and clean up pending sends and message sessions transactionally.

flotob and others added 2 commits July 12, 2026 12:43
A SafeMessage session was keyed only by Safe index and hash: a second
dApp tab requesting the identical digest resumed (and could complete)
another site's session, and a different digest silently replaced a
still-live request while its board and promise stayed attached to the
old ceremony.

Sessions now carry an unguessable token (crypto.randomUUID) plus the
requester identity {origin, webContentsId}, threaded from the dApp
provider through preload/IPC:

- sign/complete/cancel require the token; state queries with a foreign
  token render as "nothing open" for that caller
- the identical request only resumes for the SAME page (origin AND
  webContents); any other request is refused with SAFE_MESSAGE_EXISTS
  while a live session exists — no silent replacement
- sessions are dropped when the requesting webContents navigates or is
  destroyed (a dead page's leftover no longer blocks, and its
  signatures cannot linger for whatever loads next)
- the session slots live in a new dependency-light message-sessions
  module so lifecycle owners can force-drop without the signing stack

Fixes PR #160 review finding P1.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
safe-pending.json entries and live SafeMessage sessions are keyed by
wallet index. Deleting a Safe left them behind; since new wallet
indexes are assigned max+1, deleting the highest-index Safe let the
next account inherit — or be blocked by — the dead Safe's half-signed
SafeTx and session.

deleteDerivedWallet now discards the pending SafeTx entry and
force-drops the message session (unhooking its webContents listeners)
before the record is removed, via two dependency-light lazy requires.
The renderer's delete confirm warns when a waiting transaction and its
collected signatures are about to be discarded.

Fixes PR #160 review finding P2.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@flotob

flotob commented Jul 12, 2026

Copy link
Copy Markdown
Collaborator Author

Both review findings are addressed on this branch.

P1 — SafeMessage sessions identified only by Safe index/hash → fixed in 24ef088

Issue: Sessions were keyed by Safe index with the SafeMessage hash as identity. A second dApp tab requesting the identical digest resumed (and could complete) another site's session; a different digest silently replaced a still-live request while the first page's promise and board stayed attached to the wrong ceremony.

Fix: Every session now carries an unguessable token (crypto.randomUUID) plus the requester identity {origin, webContentsId}, threaded from dapp-provider.js through preload/IPC:

  • sign/complete/cancel require the token (single requireSession chokepoint); state with a foreign token renders as "nothing open" for that caller.
  • An identical digest only resumes for the same page (same origin and same webContents). Any other request — same digest from another tab, or a new digest — is refused with SAFE_MESSAGE_EXISTS while a live session exists; nothing is silently replaced.
  • Sessions are dropped when the requesting webContents is destroyed or navigates (main frame), so a dead page's leftover neither blocks new requests nor leaves collected signatures around for whatever loads next. A leftover whose webContents is provably gone may be replaced.
  • Session slots moved to a dependency-light message-sessions.js so lifecycle owners can force-drop without loading the signing stack.

P2 — Safe deletion left safe-pending.json and live sessions behind → fixed in ec746ff

Issue: Pending SafeTxs and message sessions are keyed by wallet index; indexes are assigned max+1, so deleting the highest-index Safe let the next account inherit — or be blocked by — the dead Safe's half-signed state.

Fix: deleteDerivedWallet discards the Safe's pending entry and force-drops its message session (unhooking webContents listeners) before the record is removed; a signature ceremony that lands afterwards is dropped by the existing identity re-check in signature-collection.js. The delete confirm in wallet settings now warns when a waiting transaction and its collected signatures are about to be discarded.

Tests (all passing; the new cases fail against the previous implementation)

  • safe-messages.test.js (28): same digest from a different page/tab does not resume; new request refused while one is live (no silent replace); dead-page leftover replaced; wrong/missing token rejected on sign/complete/cancel and invisible to state; navigation/destroy drops the session; lifecycle listeners unhooked on completion; stale token cannot cancel a successor session.
  • identity-manager.test.js (26): deleting a Safe clears its safe-pending.json entry and cancels its session; an account reusing the freed index starts clean.
  • Full unit suite: 119 suites, 2343 passed. Safe fork integration suite (real anvil forks, incl. on-chain isValidSignature): 5 passed with the new token API.

The inherited bridge-origin blocker from #159 is tracked on #159 itself.

@flotob flotob left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review of head ec746ff7: both original state-isolation findings are resolved. SafeMessage sessions now use an unguessable token plus origin/webContents identity, reject concurrent foreign callers, and die with the requester; Safe deletion clears pending transaction and message-session state before index reuse. The identity-manager tests pass locally; the SafeMessage suite could not load here because this checkout's node_modules lacks @safe-global/protocol-kit, while GitHub's isolated run is green.

One end-to-end caller-binding gap remains:

  • [P1] Drop Safe signing responses after the requesting document navigates (src/renderer/lib/dapp-provider.js:243). Main drops the session on did-navigate, but handleProviderRequest has no document generation and always calls sendProviderResponse(webview, id, ...) after the async signing path. If navigation lands after safeMessageComplete returns the signature but before this send, the response goes to the replacement document in the same webview; provider request ids restart per document, so it can satisfy a reused id. Bind the renderer request to a webview document generation (as #145 now does) and suppress both result and error delivery after navigation/destruction. Add a deferred completion test covering that boundary.

This head also inherits #159's unresolved production bridge-origin blocker and does not yet contain #159's latest distribution guard; refresh it after #159 changes. Current status: 36/36 checks successful.

flotob and others added 2 commits July 12, 2026 14:09
Refresh the branch from its base so it carries the dist guard added to
scripts/build.js on feature/openlv (15ab32d).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Main drops Safe message sessions on navigation, but the renderer's
handleProviderRequest still delivered the result (or error) of an
in-flight async request into whatever document currently occupies the
webview. Provider request ids restart per document, so a signature
completing after a navigation could satisfy a reused id in the
replacement page.

Mirror the document-generation guard from the Radicle provider (#145):
a per-webview generation in a WeakMap, bumped on did-navigate and
destroyed, captured when a request arrives, and re-checked before both
the success and error sends — stale responses are silently dropped.
The guard covers every async provider path, not just Safe signing.

Tests cover deferred Safe personal_sign completion after navigation
(result and error paths), destruction mid-request, and that a fresh
request from the replacement document still gets its response.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@flotob

flotob commented Jul 12, 2026

Copy link
Copy Markdown
Collaborator Author

Round-2 finding addressed in ba359e4.

[P1] Renderer-side stale response delivery — confirmed. Main dropped the SafeMessage session on did-navigate, but handleProviderRequest still called sendProviderResponse unconditionally after the async signing path, so a signature (or error) completing after a navigation could land in the replacement document and satisfy a reused request id.

The fix mirrors the document-generation guard from #145's Radicle provider: setupWebviewProvider now keeps a per-webview navigation generation in a WeakMap, bumped on did-navigate and destroyed. handleProviderRequest captures the generation at request arrival and re-checks it before both the success and the error send — stale responses are silently dropped. The guard covers every async provider path, not just Safe signing.

New tests in src/renderer/lib/dapp-provider.test.js (all verified to fail against the previous code): deferred Safe personal_sign completion after navigation (result and error paths), webview destruction mid-request, plus baselines that same-document delivery and a fresh post-navigation request still work. Targeted suites (src/main/wallet/safe, src/renderer/lib) pass: 39 suites, 723 tests.

The branch was also refreshed from feature/openlv (merge f9c9010), so it now carries the #159 dist guard on scripts/build.js. The #159 bridge-origin blocker itself remains tracked there.

@flotob flotob left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-re-review of head ba359e48 against ec746ff7: stale result/error delivery is resolved. The per-webview generation now suppresses both response paths after committed navigation/destruction, the new tests cover result, error, destruction, and a fresh replacement-document request, and the targeted provider/identity suites pass locally (31/31). This head also now contains #159's distribution guard.

One related chrome-lifecycle gap remains:

  • [P2] Withdraw the Safe signing board when its requesting document navigates (src/renderer/lib/wallet/safe-signing.js:109). Main discards the SafeMessage session and the provider now drops its eventual response, but nothing settles/closes openSafeMessageBoard or hides its browser-chrome UI. The old site's signing board, origin summary, collected-signature state, and Ledger polling remain visible after navigation until the user interacts; that action then fails because the session is already gone. Route the owner-scoped document invalidation to an abandon/close hook for this board, without closing a successor tab's board, and add a navigation test.

The inherited #159 production-origin blocker also remains. 36/36 checks successful.

Round 2 bound provider responses to the requesting document and main
already drops the SafeMessage session on navigation — but the board
chrome survived: the old site's signing board, origin summary, and
Ledger polling stayed visible until the user interacted, and that
action then failed because the session was gone.

openSafeMessageBoard now records the requesting webview as the board's
owner, and the provider's existing document-invalidation seam
(did-navigate/destroyed) fans out to a new abandonSafeMessageBoard
hook: it rejects the pending dApp promise (the generation guard drops
the stale rejection), stops Ledger detection, and takes the screen
down. The hook is owner-scoped — mirroring radicle-consent's
dismissal — so a successor document's board is never closed by a
predecessor's invalidation.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@flotob

flotob commented Jul 12, 2026

Copy link
Copy Markdown
Collaborator Author

Round-3 finding addressed in 1c7bb95.

[P2] Withdraw the Safe signing board when its requesting document navigates — confirmed: main dropped the session and round 2 suppressed the response, but the board chrome (origin summary, signature rows, Ledger polling) stayed up until the user interacted, only to fail against the already-gone session.

openSafeMessageBoard now records the requesting webview as the board's owner, and the provider's existing invalidateDocument seam (did-navigate/destroyed) fans out to a new abandonSafeMessageBoard(owner) hook in safe-signing.js: it rejects the pending dApp promise with the same 4001 user-rejection as the cancel path (the round-2 generation guard drops that stale rejection before it reaches the page), stops Ledger detection, and takes the screen down. The hook is owner-scoped, mirroring dismissRadicleConsent from #145, so an invalidation from a predecessor document never closes a board opened by a successor document or another tab.

Tests: new safe-signing.test.js covers navigation withdrawal (board hidden, promise settled, Ledger polling stopped), predecessor/successor owner scoping, and the unchanged user-cancel and threshold-met success paths; dapp-provider.test.js verifies the board is opened with its webview as owner and withdrawn on did-navigate/destroyed. The navigation tests fail against the previous commit. Full src/renderer/lib + src/main/wallet/safe suites pass (40 suites, 729 tests).

The inherited #159 origin blocker remains tracked on #159.

@flotob flotob left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-re-re-review of head 1c7bb954 against ba359e48: the reported idle-board lifecycle issue is fixed. Navigation/destruction now withdraws the owning webview's board, settles its promise, hides the chrome, and stops Ledger polling; the new targeted suites pass locally (11/11), and CI is green (36/36).

One cross-session race remains in the new abandonment path:

  • [P1] Fence in-flight board operations before a successor session can reuse the globals (src/renderer/lib/wallet/safe-signing.js:319). abandonSafeMessageBoard clears the current resolver/token and hides the board, but an earlier signOwner or completeNow await is not cancelled or generation-checked. If document A's safeMessageComplete is pending, A navigates, and document B opens a successor board before A's IPC continuation resumes, A's success path calls the module-global settleMessage and resolves B's promise with A's signature. A delayed sign/error path can likewise overwrite B's signingIndex, state, notes, or polling. Give each board/session an epoch or entry object, invalidate it on abandon/open, and require the captured epoch after every await before mutating state or settling; add a deferred old-completion test that opens a successor before resolving the predecessor.

The predecessor/successor test added here uses two distinct owner objects, while production passes the persistent webview object, and it does not exercise any in-flight continuation. The inherited #159 production-origin blocker also remains.

Abandoning the Safe signing board settled its promise and hid the
screen, but an in-flight IPC await (safeMessageComplete, a sign
attempt, refreshState, Ledger probing, execute, discard) resumed
afterwards against the module globals — so a predecessor document's
late completion could resolve a successor board's promise with the
wrong signature, or overwrite its state, row notes, and polling.

Every board session now takes a fresh epoch on open, and abandoning
a message board bumps it too; every async path captures the epoch
before awaiting and bails after each await once it is stale — no
settle, no state mutation, no UI writes, no polling restart. The
best-effort cancel IPC still fires; only the continuation is fenced.

Tests hold a predecessor's completion and sign replies genuinely in
flight, reuse the SAME persistent owner object production passes (the
webview), and assert the successor's promise, board HTML, session
token, and Ledger polling stay untouched while its own completion
still settles normally. The fake DOM now matches bare [data-*]
selectors so the tests can drive the real signOwner click wiring.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@flotob

flotob commented Jul 12, 2026

Copy link
Copy Markdown
Collaborator Author

Round 4 addressed in 05f82a3.

[P1] Cross-session race — confirmed and fixed. You were right: abandonSafeMessageBoard settled the promise and hid the board, but an in-flight await (safeMessageComplete, a sign attempt, refreshState, Ledger probing, execute, discard) resumed against the module globals afterwards — a predecessor's late completion could resolve a successor board's promise with the predecessor's signature, and a delayed sign error could write into the successor's state, notes, and polling. Writing the tests first reproduced both: B settled with A's signature, and A's error path even hit main with B's session token.

The fix is a board-session epoch (boardEpoch + isStale(epoch) in safe-signing.js): every openBoard takes a fresh epoch, and abandonSafeMessageBoard bumps it too. Each async path captures the epoch before its first await and re-checks it after every await — when stale it bails with no settle, no state mutation, no UI write, and no polling restart. Fenced paths: openBoard, refreshState, requestVaultUnlock, progressAutomatics, signOwner (including the vault-unlock branch), completeNow, executeNow, handleDiscard, and the Ledger detection tick. The abandon path's best-effort cancel IPC still fires — only its continuation is fenced.

Tests strengthened as requested. The new tests pass the SAME persistent owner object for predecessor and successor (matching production, which hands the board the webview for every document it hosts) and hold the predecessor's IPC replies genuinely in flight via deferred mocks: (a) A's held completion resolving after B opens cannot settle B or touch its board HTML / Ledger polling; (b) A's delayed sign error (driven through the real signOwner click wiring) cannot write a row note, re-render, or refresh against B's token; (c) B's own held completion still settles B normally afterwards; the round-3 owner-scoping test is unchanged and still green. All three new tests fail on the previous commit. (test/helpers/fake-dom.js gained bare [data-*] selector matching to drive the click path.)

The inherited #159 origin-binding blocker remains tracked on #159.

@flotob flotob left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review of head 05f82a32 against 1c7bb954: the cross-session continuation race is resolved.

The board epoch advances on every open and owner-scoped abandonment, and the asynchronous state machine consistently checks its captured epoch after each await before settling a promise, mutating shared state/UI, retrying, refreshing, or restarting Ledger polling. I specifically traced the completion-success handoff, signing success/error and vault retry paths, refresh/finalization, execute/discard, and in-flight Ledger probe cases; a predecessor can no longer adopt the successor's resolver, token, state, notes, or timers.

The strengthened tests now use the same persistent webview owner and defer predecessor completion/signing replies until after a successor opens. They cover the original signature-misdelivery and stale-error mutations, plus normal successor completion. Targeted suites pass locally (14/14), and CI is green (36/36).

No remaining PR-specific blocking findings. The stacked PR still inherits #159's separately tracked production bridge-origin blocker.

flotob and others added 5 commits July 12, 2026 16:21
Pin the new release together with the SHA256SUMS trust digest. This picks up the upstream phantom postage batch fixes while preserving reproducible binary downloads.
Pin the new release together with the SHA256SUMS trust digest. This picks up the upstream phantom postage batch fixes while preserving reproducible binary downloads.
chore(build): update bundled Ant to v0.5.41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants